Skip to content

Conversation

@Ecarrion
Copy link
Contributor

@Ecarrion Ecarrion commented Jan 5, 2023

Closes: #8492

Why

This PR asks the merchant for confirmation before generating multiple variations remotely.

How

  • Refactor ViewModel.generateAllVariations method to provide an onStateChanged closure that will inform the consumer what is happening in the generation process.

  • Update View controller to listen to those state changes.

  • Presents confirmation alert when the state change indicates.

Demo

demo.mov

Testing Step

  • Go to a variable product
  • Add some attributes and options to the product
  • Start the "Generate All Variations" flow
  • See that an alert is presented to the merchant to confirm the generation process.

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@Ecarrion Ecarrion added the feature: variation list Related to the variations list for variable products. label Jan 5, 2023
@Ecarrion Ecarrion added this to the 11.8 milestone Jan 5, 2023
@Ecarrion Ecarrion requested a review from ealeksandrov January 5, 2023 22:26
@wpmobilebot
Copy link
Collaborator

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr8565-cbc110b on your iPhone

If you need access to App Center, please ask a maintainer to add you.

Base automatically changed from issue/refactor-results-controller to trunk January 6, 2023 14:52
Copy link
Contributor

@ealeksandrov ealeksandrov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

let format = NSLocalizedString("This will create a variation for each and every possible combination of variation attributes (%d variations).",
comment: "Alert description to allow the user confirm if they want to generate all variations")
return String.localizedStringWithFormat(format, variationCount)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice wrapper!


self.createVariationsRemotely(for: product, variations: variationsToGenerate, onCompletion: onCompletion)
// Confirm generation with merchant
onStateChanged(.confirmation(variationsToGenerate.count, { confirmed in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is hard to read and understand, but I don't have easy improvement suggestions 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, not a fan either. will keep it in the back of my head for improvements!

@Ecarrion Ecarrion merged commit 0758361 into trunk Jan 6, 2023
@Ecarrion Ecarrion deleted the issue/8492-inform-variations-alert branch January 6, 2023 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: variation list Related to the variations list for variable products.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Variations: Inform about variations to be created

4 participants